-
Notifications
You must be signed in to change notification settings - Fork 5
Exact R2 Indicator #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
I wouldn't worry about the MATLAB interface unless you are planning to use it yourself. |
| ASSUME(dim == 2); | ||
|
|
||
| // p is sorted by f1 (primarily), then f2 (secondarily) | ||
| const double **p = generate_sorted_doublep_2d(data, &n, DBL_MAX); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to create a variant of generate_sorted_doublep_2d that filters points such that p[j][0] < ref[0]. Then you can delete the while loop below.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
==========================================
+ Coverage 77.91% 78.01% +0.09%
==========================================
Files 62 63 +1
Lines 5878 5949 +71
Branches 901 913 +12
==========================================
+ Hits 4580 4641 +61
- Misses 1100 1103 +3
- Partials 198 205 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
See the tests in test_moocore.py. I would add similar tests to those added for epsilon/igd/hypervolume. |
|
If tests are longer than .5 s, it may be a good idea to create a command-line interface (see epsilon.c) and add the tests to https://github.com/multi-objective/testsuite |
|
To give it more visibility, you may want to extend the examples here: https://github.com/multi-objective/moocore/blob/main/python/examples/plot_metrics.py or add its own example showcasing why/when people should use this R2 instead of other metrics. This can be done as a follow-up (but it will also provide additional testing so it may be a good idea to do it now). |
This comment was marked as resolved.
This comment was marked as resolved.
|
This looks great. Are you testing what happens if the ref point is dominated and nondominated? I may have missed it. Also, could you squash all commits into one? I can do it if you don't know how, but it will look better in the logs if you do it. |
|
All tests passed so let me know if you want to change anything else and if you want me to squash the commits, before I merge this. |
52fd658 to
c727a08
Compare
|
I've added the mathematical details and squashed the commits now! One thing you could add is the original source of the R2 indicator to the documentation, as I don't find it in the linked bib-files and cannot easily add it myself: @techreport{hansen1998,
title={{Evaluating the Quality of Approximations to the Non-dominated Set}},
author={Hansen, Michael Pilegaard and Jaszkiewicz, Andrzej},
year={1998},
institution={IMM, Department of Mathematical Modelling, Technical University of Denmark},
number = {IMM-REP-1998-7}
}Thanks! |
https://iridia-ulb.github.io/references/#HanJas1998 You just need to add the label to bibkeys.txt and run update_bib.sh and it will fetch it from the iridia-ulb GitHub repo. |
Squash of the following 30 commits: Add files for exact R2 indicator Basic bi-objective R2 indicator implementation Skip points not dominated by ideal ref and remove logging Provide basic documentation for exact R2 Add as contributor Add exact_r2 as news [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Remove empty section to make pre-commit-ci happy Rename exact_r2 -> r2_exact [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Optimizations in r2_exact computation Fix citation Skip another unnecessary check Improve edgecase handling Inline subfunction call [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Add functional tests for r2_exact [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Remove unused object Add exact R2 to metrics example [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Remove additional space (which messed up the layout) Use correct r2_exact function name Fix docstring formatting in test_r2_from_doc Update reference argument in r2_exact examples Fix precision of R2 exact function output Refactor _utility function and variable declarations [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Add mathematical description of R2 Remove () Add original source of R2 indicator
I figured out how to use the script already, I just did not find that reference in the files 😁 ... Anyway, it worked now! That should be all for this PR, I think :-) |
Hey Manuel (et al.),
I have finally implemented the exact R2 indicator (for bi-objective solution sets) in the C library and added the Python interface for it. The code is mostly based on the hypervolume function for two objectives.
I have also added basic documentation for the exact_r2 function in the Python package.
For rudimentary testing, I have attached a Jupyter notebook: tests.ipynb
For a full integration of the exact R2 indicator, there's still the following todos:
Further R2-related functions could be implemented:
I'm still getting familiar with developing for the library, so I'm opening a PR for the current state of my implementation :-)
Let me know how I can help to achieve a quick turnaround with the merge!
Cheers,
Lennart